Skip to content

[FEATURE]Support Dynamic Config.#4072

Open
LiangDai-Mars wants to merge 1 commit intoapache:masterfrom
LiangDai-Mars:apache-support-dynamic-config
Open

[FEATURE]Support Dynamic Config.#4072
LiangDai-Mars wants to merge 1 commit intoapache:masterfrom
LiangDai-Mars:apache-support-dynamic-config

Conversation

@LiangDai-Mars
Copy link
Contributor

Why are the changes needed?

Close #xxx.

Brief change log

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

* @param properties plugin properties
* @param configurationManager configuration manager used to fetch dynamic overrides
*/
default void open(Map<String, String> properties, ConfigurationManager configurationManager) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using void open(Configuration configs) instead.

* Configurations} is expected. All getter methods defined in {@link Configurations} operate on the
* current merged configuration.
*/
public class DynamicConfigurations extends Configurations {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DynamicConfigurations and ConfigurationManager should be in ams module.

Comment on lines +96 to +101
public TerminalManager(
org.apache.amoro.config.DynamicConfigurations dynamicConfigurations,
CatalogManager catalogManager) {
this((Configurations) dynamicConfigurations, catalogManager);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. No need type casting.
  2. Import at file head.

}

public ProcessService(
org.apache.amoro.config.DynamicConfigurations dynamicConfigurations,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import class at file head.

Comment on lines 80 to +82
public AbstractPluginManager(String pluginCategory) {
this(pluginCategory, null, null);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this constructor may should be removed.

@LiangDai-Mars LiangDai-Mars force-pushed the apache-support-dynamic-config branch 2 times, most recently from bbb5292 to 3d670bf Compare February 13, 2026 08:33
@github-actions github-actions bot added the type:docs Improvements or additions to documentation label Feb 13, 2026
@LiangDai-Mars LiangDai-Mars force-pushed the apache-support-dynamic-config branch from 3d670bf to a18ae96 Compare February 13, 2026 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module module:common type:docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants